JSpecify: handle Nullability for return types of lambda expressions for Generic Types. #854
Conversation
…essions for generic types
testForLambdaReturnTypeInAnAssignment
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #854 +/- ##
============================================
+ Coverage 86.92% 86.93% +0.01%
- Complexity 1883 1886 +3
============================================
Files 74 74
Lines 6210 6215 +5
Branches 1206 1208 +2
============================================
+ Hits 5398 5403 +5
Misses 405 405
Partials 407 407
☔ View full report in Codecov by Sentry. |
| } else if (config.isJSpecifyMode() | ||
| && GenericsChecks.getGenericMethodReturnTypeNullness( | ||
| methodSymbol, ASTHelpers.getType(tree), state, config) | ||
| .equals(Nullness.NULLABLE)) { | ||
| // Get the Nullness if the Annotation is indirectly applied through a generic type if we | ||
| // are in JSpecify mode | ||
| return Description.NO_MATCH; |
There was a problem hiding this comment.
This code won't work for the case when the lambda is written x -> { return null; }. I updated the test and we get a false positive. The issue is that with the return null case, the tree passed in is the ReturnTree, not the tree for the lambda itself. One way to fix this would be to add another parameter @Nullable Tree lambdaTree to checkReturnExpression and then only run this case when lambdaTree is non-null. The extant tree parameter can be renamed to errorTree, since its main purpose is for finding the right locations to report errors.
For the negative scenario mentioned above where we do not expect an exception, we were getting a false positive. After computing the Nullability correctly when it is indirectly applied through the generic type we no longer get this false positive.
All the test cases in NullAwayJSpecifyGenericTests.java have passed for these changes.
Similar changes for handling lambda parameters were made under #852